-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Cleanup should_implement_trait
lint code
#15804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup should_implement_trait
lint code
#15804
Conversation
Profile results: +0.04% performance difference on tokio-1.38.1, the lint itself grew 11k -> 31.6k |
Could also try Then again, 0.04% seems so small that it may as well just be noise, right? Don't know much effort would be worth putting into optimizing this then |
I profiled again |
The other thing I wanted to try after this is a binary search indeed (an explicit one), but this requires the array to be sorted, and there is no const-stable sort function as far as I can see (to do that at compile time). How would LLVM not to a linear scan in a non-sorted array? |
If we don't get even better performances, I'll just keep the cleanups and restore the linear scan we had if it is that cheap. |
It unrolls the loop so that it's just a sequence of cmps and jmps (getting rid of the array entirely), then it can freely reorder the compare instructions around to do a binary search. Example: https://godbolt.org/z/Kdfvx6zKc (heavily simplified but I've verified locally that LLVM does the same on clippy with |
a1182d2
to
cd37148
Compare
Let's check whether it gives better performances or not. In any case, it should not be worst and is cleaner. |
803f4e3
to
e832c0c
Compare
e832c0c
to
87d2891
Compare
should_implement_trait
lint code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! ❤️
It still gives some worst performance (7.5k -> 11k), but it's in the noise range and it could be changed at any moment by compiler optimizations or the user having another program open. |
This might positively affect performances of the
should_implement_trait
lint.r? blyxyas
changelog: none